Skip to content

Fix ISY moisure sensors showing unknown until a leak is detected#14496

Merged
MartinHjelmare merged 6 commits intohome-assistant:devfrom
OverloadUT:fix-13384-isy-moisure-sensors
May 21, 2018
Merged

Fix ISY moisure sensors showing unknown until a leak is detected#14496
MartinHjelmare merged 6 commits intohome-assistant:devfrom
OverloadUT:fix-13384-isy-moisure-sensors

Conversation

@OverloadUT
Copy link
Copy Markdown
Contributor

@OverloadUT OverloadUT commented May 16, 2018

Description:

ISY moisture sensors currently show as "Unknown" state until they detect a leak or the user presses the button manually on the sensor. This was due to some obnoxiously nuanced ways that the moisture sensor sends signals. This PR fixes all of the ways that it could show up wrong, including a very specific edge case that can happen for the first 24 hours after the ISY controller reboots. They should be perfect now!

Related issue (if applicable): fixes #13384

Pull request in home-assistant.github.io with documentation (if applicable): n/a

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

Added some logic that handles both moisture sensors and door/window sensors
If a leak sensor is unknown, due to a recent reboot of the ISY, the status will get updated to dry upon the first heartbeat. This status update is the only way that a leak sensor's status changes without an accompanying Control event, so we need to watch for it.
State was checking the incorrect parameter, and wasn't calling schedule update
@@ -191,12 +197,21 @@ def _positive_node_control_handler(self, event: object) -> None:

# pylint: disable=unused-argument
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused argument is already globally disabled.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

pass
if self._status_was_unknown and self._computed_state is None:
# pylint: disable=protected-access
self._computed_state = bool(self._node.status._val)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to access the private attribute _val to get the value? It seems weird that the only way to access the value would by a private attribute. What does _node.status return?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the status object here is odd. It's a unique VarEvents object that is meant to clone all of the functionality of a built-in type, but with the added ability to have code subscribe to updates to its value.

While I can access self._node.status and it shows up as a string '0' or '255', trying to cast it to a bool causes the code to silently halt with no exception and I cannot figure out why.

In the end I decided to go with what the base ISYDevice class does in its value() method, which was, in fact, originally implemented that way by the creator of the VarEvents package.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does bool(int(self._node.status)) work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested that, and that does indeed work! I pushed it up just now because I suppose it's slightly better than accessing a protected property, even though it is a bit of a hack

We can't cast _.status directly to a bool for some unknown reason (possibly with the VarEvents library), but casting to an int then bool does work.
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐝

@MartinHjelmare MartinHjelmare merged commit 23afdec into home-assistant:dev May 21, 2018
@MartinHjelmare
Copy link
Copy Markdown
Member

Should it be tagged for 0.70?

@OverloadUT
Copy link
Copy Markdown
Contributor Author

It's a bugfix, so I think it could be. (It wasn't introduced by 0.70 though)

@MartinHjelmare MartinHjelmare added this to the 0.70.0 milestone May 21, 2018
balloob pushed a commit that referenced this pull request May 24, 2018
)

* Fix ISY leak sensors always showing UNKNOWN until a leak is detected

Added some logic that handles both moisture sensors and door/window sensors

* Handle edge case of leak sensor status update after ISY reboot

If a leak sensor is unknown, due to a recent reboot of the ISY, the status will get updated to dry upon the first heartbeat. This status update is the only way that a leak sensor's status changes without an accompanying Control event, so we need to watch for it.

* Fixes from overnight testing

State was checking the incorrect parameter, and wasn't calling schedule update

* Remove leftover debug log line

* Remove unnecessary pylint instruction

* Remove access of protected property

We can't cast _.status directly to a bool for some unknown reason (possibly with the VarEvents library), but casting to an int then bool does work.
@balloob balloob mentioned this pull request May 28, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
…e-assistant#14496)

* Fix ISY leak sensors always showing UNKNOWN until a leak is detected

Added some logic that handles both moisture sensors and door/window sensors

* Handle edge case of leak sensor status update after ISY reboot

If a leak sensor is unknown, due to a recent reboot of the ISY, the status will get updated to dry upon the first heartbeat. This status update is the only way that a leak sensor's status changes without an accompanying Control event, so we need to watch for it.

* Fixes from overnight testing

State was checking the incorrect parameter, and wasn't calling schedule update

* Remove leftover debug log line

* Remove unnecessary pylint instruction

* Remove access of protected property

We can't cast _.status directly to a bool for some unknown reason (possibly with the VarEvents library), but casting to an int then bool does work.
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018
@OverloadUT OverloadUT deleted the fix-13384-isy-moisure-sensors branch May 2, 2019 20:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isy994 moisture sensors unknown at startup

4 participants